feat(react-headless-components-preview): add Popover & positioning#36006
feat(react-headless-components-preview): add Popover & positioning#36006mainframev wants to merge 18 commits intomicrosoft:masterfrom
Conversation
f26ae3e to
22f36ba
Compare
|
Pull request demo site: URL |
22f36ba to
e9fef9d
Compare
📊 Bundle size report✅ No changes found |
58218ca to
c32d139
Compare
c32d139 to
4774cce
Compare
4774cce to
0022e71
Compare
|
|
||
| export type PositioningReturn = { | ||
| targetRef: React.RefCallback<HTMLElement>; | ||
| containerRef: React.RefCallback<HTMLElement>; |
There was a problem hiding this comment.
should this be surfaceRef or is it for consistency with v9?
There was a problem hiding this comment.
yes, for consistency
| function mountHook(options: PositioningProps = {}) { | ||
| const resultRef = React.createRef<{ current: PositioningReturn }>(); | ||
| const Capture = () => { | ||
| const result = usePositioning(options); |
There was a problem hiding this comment.
nit, but we can probably type-cast once if it's really needed
| const result = usePositioning(options); | |
| const result = usePositioning(options) as unknown as { current: PositioningReturn }; |
| result.current.containerRef(node); | ||
|
|
||
| expect(node.style.getPropertyValue('position-anchor')).toMatch(/^--popover-anchor-/); | ||
| expect(node.style.getPropertyValue('position-area')).toBe('block-end span-inline-end'); |
There was a problem hiding this comment.
nit, but we can use the https://github.com/testing-library/jest-dom#tohavestyle to make it a bit cleaner
| expect(node.style.getPropertyValue('position-area')).toBe('block-end span-inline-end'); | |
| expect(node).toHaveStyle({ positionArea: 'block-end span-inline-end' }); |
| const node = document.createElement('div'); | ||
| result.current.containerRef(node); | ||
|
|
||
| expect(node.style.position).toBe('absolute'); |
There was a problem hiding this comment.
nit, but can be combined to one assertion for all styles
| const childProps = (child?.props ?? {}) as Record<string, unknown>; | ||
|
|
||
| const triggerChildProps = { | ||
| 'aria-expanded': `${open}` as 'true' | 'false', |
There was a problem hiding this comment.
Why don't we use the popovertarget attribute https://developer.mozilla.org/en-US/docs/Web/API/Popover_API#html_attributes?
There was a problem hiding this comment.
current implementation uses showPopover() and I found it more suitable as we need to also show Popover on hover (v9 canonical Popover has that feature), have it controlled and uncontrolled, mouseLeaveDelay for openOnHover etc
|
|
||
| const positioning = usePositioning(resolvePositioningShorthand(props.positioning)); | ||
|
|
||
| useOnClickOutside({ |
There was a problem hiding this comment.
just to confrim, did you check if Popover API has this functionality out-of-the-box?
There was a problem hiding this comment.
it does, but only when popover has popover='auto' / popover attribute, which has native dismiss on Esc and overlay click, great that it works also with nested popovers, but in current implementation it uses manual, which does not have such functionality out of the box, because we need multiple non-nested Popovers to be opened. I think auto will be a great fit to Tooltip
There was a problem hiding this comment.
We may need to set auto as the default option and offer a way to switch to manual + custom logic when advanced features are required. I'm not entirely sure how this would work yet, but we might need a similar approach for this and positioning as well
There was a problem hiding this comment.
maybe we should split this into two separate components (as subcomponents of Popover). Embedding logic like useOnClickOutside and useOnScrollOutside directly in hooks doesn't seem appropriate, especially when auto is only used to support dismissing an already open Popover. This approach as a default also diverges from v9 Popover behavior, where multiple Popovers can be open simultaneously
773c5b6 to
6aea7b4
Compare
…n native CSS anchor positioning
…n spec-pure CSS anchor positioning
…and unsupported positioning props
…ation-1 strip-down
b5ae6c5 to
13c85ad
Compare
…assertions to toHaveStyle
Adds a PopoverAuto component that renders its surface with `popover="auto"` instead of `popover="manual"`, so light dismiss (Escape, click-outside, popover-stack peer dismissal) is handled by the browser rather than React. Implementation: - Extracts a shared internal hook `useInternalPopover(props, popoverType)` in usePopover.ts. Both the existing `usePopover` (manual) and a new `usePopoverAuto` are thin wrappers that fix the popoverType. - Sets the `popover` attribute to the actual mode value when calling `showPopover()`. - In auto mode: disables `useOnClickOutside` / `useOnScrollOutside` and attaches a `toggle` event listener on the surface to mirror the browser's open/closed state into React and fire onOpenChange. - Plumbs `popoverType` through PopoverContextValue so PopoverSurface can skip its own Escape handler in auto mode (the browser owns it). - Exports `PopoverAuto` and `usePopoverAuto` from the popover subpath. - Updates the Nested Storybook story to use PopoverAuto, where browser popover-stack semantics keep ancestor surfaces open when a descendant popover opens, and dismiss the ancestor chain on outside interaction. Tests, type-check, lint, and api-report all pass.
…om usePopover/usePopoverAuto
6e877df to
ce13da9
Compare
…er the auto Popover surface (re)mounts
ce13da9 to
68886c7
Compare
Adds a headless Popover (Popover, PopoverTrigger, PopoverSurface) to the @fluentui/react-headless-components-preview package, positioned via the native CSS Anchor Positioning API instead of floating-ui
Feature with v9
@fluentui/react-popoveropen/defaultOpen/onOpenChangeopenOnHover+mouseLeaveDelayopenOnContext(right-click, cursor-anchored)withArrow(+ consumer-owned arrow CSS via[data-placement])trapFocus+aria-modal/role="dialog"disableAutoFocuscloseOnScroll,closeOnIframeFocusinline(skip top-layer)<Portal>wrapper - DOM placement only, positioning math unchanged. Headless: skips HTML Popover API top-layer promotion, also changes the overflow boundary CSSposition-try-fallbacksflips against (viewport → nearest + scroll port / containing block)mountNode(portal target)positioning.position+positioning.alignpositioning.offset(number or{ mainAxis, crossAxis })positioning.coverTargetpositioning.fallbackPositionspositioning.autoSize(true/'width'/'height')positioning.overflowBoundarypositioning.overflowBoundaryPaddingpositioning.overflowBoundaryRectpositioning.matchTargetSize: 'width'positioning.strategy: 'absolute' | 'fixed'positioning.pinnedpositioning.target(custom anchor)positioning.positioningRef(imperativesetTarget)flipBoundaryarrowPaddingshiftToCoverTargetonPositioningEnddisableUpdateOnResizeuseTransformRelated Issue(s)